refactor(date-field): align DateField and DatePicker with Field composition#392
refactor(date-field): align DateField and DatePicker with Field composition#392IzumiSy wants to merge 1 commit into
Conversation
045166b to
ccdac38
Compare
interacsean
left a comment
interacsean
left a comment
There was a problem hiding this comment.
Great to start to align the interface with the rest of the form-based components we have already and with RHF. We should also bring updates to the Calendar component to keep them all aligned.
Ontop of this, I am concerned about whether we should be maintaining backwards compatibility at all and give future deprecation warnings, or try to survey if any of these components are in use already if we want to make a breaking change
- The field is not working within a Form. In the example page, there is a form to test this functionality and it has broken... AI analysis says:
"The refactor registers the date control as a Base UI form field via the internal useRegisterFieldControl. Base UI's Form.onSubmit only calls your onSubmit/onFormSubmit when zero registered fields are invalid; otherwise it preventDefault()s and focuses the first "invalid" field (Form.js, the if (invalidFields.length) branch). A field's validity is !invalid && validityData.state.valid, and validityData.state.valid starts at null and is only committed via queueMicrotask, gated behind shouldValidateOnChange() (date-field.tsx:156). Under the default validationMode="onSubmit" it never resolves to true, so the date field is permanently counted invalid"
A typed out-of-range / unavailable date shows a red border but no message, the control does its own validation internally but never reports that validity back up to Field.Root.
- Existing form components have { readOnly?: boolean, disabled?: boolean }
mode="editable" | "readonly" | "disabled" does not align, and should become two independent booleans
disabled and readOnly are orthogonal, not one axis. In HTML they mean different things — a disabled field isn't focusable, isn't submitted, and isn't validated; a read-only field is focusable, is submitted, and is validated
It breaks native/RHF interop. register() and a spread {...field} set disabled as a boolean; Base UI's own Field.Root cascades disabled as a boolean. With mode, every integration point needs a translation layer — and it's already caused a bug in this PR: Field.Root disabled flows to the group but not to calState/the state hook (they only read mode === "disabled")
-
Re constraints. Again { required?: boolean } is the existing convention, not nested. And we could shift min|maxValue to just min|max top-level props, which is how a native number-based input expects this type of range restriction. However we would be passing objects not strings so perhaps retaining Value is valid
-
There are direct imports from @base-ui/react/internals/* which is brittle as they are not semver protected, and sounds like it is linked to the form no longer submitting
5. We lost a few tests related to typing/auto-advance, day-clamping, leap years etc. Is there rationale / scope decision to this?
2 participants
Motivation
DateField/DatePickerwere the odd ones out in the form stack.The rest of AppShell already separates responsibilities cleanly:
Formis the top-level form shellField.Rootowns label / description / invalid / error presentationSelect,Combobox, andAutocompleteare exposed as bare controls that compose insideField.RootBy contrast, the date controls mixed control logic and field chrome in one component via props such as
label,description,errorMessage, andisInvalid. That made React Hook Form usage inconsistent with the rest of the library, and left the public API with multiple overlapping sources of invalid state.This PR moves the date controls onto the same composition model as the rest of the form controls.
Design Decision
Interoperability with React Hook Form
Field.Rootalready mirrors React Hook Form'sfieldStateshape (isTouched,isDirty,invalid,error), so the natural integration point in this codebase is:Field.Rootfor label / description / error presentationvalue,onChange,onBlur,name, andrefThat is already how the rest of the form stack wants to be used. The old date API blurred that boundary by letting the control also own visible error presentation through
errorMessageandisInvalid.This refactor makes the date controls work the same way as other form controls: they expose control-level behavior and include hidden proxy wiring so
Field.Label, form submission, and form libraries can still target them naturally.API Consistency with Existing Dropdown Controls
Select,Combobox, andAutocompleteare already exposed as bare controls:label/description/errorpropsaria-label,aria-labelledby,id)Field.Rootwhen field chrome is neededTo keep the library internally consistent,
DateField/DatePickershould follow that same model instead of introducing a separate "closed field" pattern just for date inputs.This PR therefore reshapes the date controls around the same ideas:
Field.RootRationale for Reshaping Flag Props
The previous surface had several overlapping boolean- and message-driven sources of state:
isInvaliderrorMessageisRequiredisDisabledisReadOnlyminValue/maxValue/isDateUnavailableThat made it easy to express ambiguous states, for example:
The new shape makes those changes explicit:
isDisabledmode="disabled"isReadOnlymode="readonly"isRequiredconstraints.requiredminValueconstraints.minmaxValueconstraints.maxisDateUnavailableconstraints.unavailableisInvalidField.Root invaliderrorMessageField.Error/Field.Root error stateIn the same spirit, field chrome moved out of the controls entirely:
labelField.LabeldescriptionField.DescriptionThis keeps the date control closer to a single source of truth model: interaction and constraints belong to the control, while field-shell presentation belongs to the field shell.
Summary
DateField/DatePickerto align with the existingForm+Field+ control architecture.mode,constraints, accessible naming,onBlur, ref/form wiring).Field.Rootcomposition and the new API.